Harden against undefined index in _load_script_textdomain_from_src() and add types for wp_parse_url()#11690
Conversation
…n _load_script_textdomain_from_src()
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| if ( ! $src_url ) { | ||
| return false; | ||
| } | ||
| $src_url['path'] ??= ''; |
There was a problem hiding this comment.
This is the crux for fixing the undefined index error.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
@manzoorwanijk Please review. |
| } | ||
|
|
||
| $path = untrailingslashit( $path ); | ||
| if ( $path ) { |
There was a problem hiding this comment.
Conditional added because the $path can still be false here since $wp_textdomain_registry->get() returns string|false.
|
|
||
| // If the source is not from WP. | ||
| if ( false === $relative ) { | ||
| if ( ! is_string( $relative ) ) { |
There was a problem hiding this comment.
Since apply_filters() can return anything, safer to check for a string specifically than for equality with false.
There was a problem hiding this comment.
Pull request overview
This PR aims to harden script translation path resolution against edge cases in URL parsing (preventing undefined index / invalid offsets) and to improve static analysis by adding PHPStan typing for wp_parse_url() and its helper.
Changes:
- Adds guards/defaults in
_load_script_textdomain_from_src()to avoid undefined index access when working with parsed URLs. - Adjusts handling of unresolved relative paths to be more type-safe (
! is_string( $relative )). - Adds PHPStan conditional return types and array-shape annotations for
wp_parse_url(),_get_component_from_parsed_url_array(), and_wp_translate_php_url_constant_to_key().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/wp-includes/l10n.php |
Adds URL-parse hardening and path handling tweaks for script/module translation resolution. |
src/wp-includes/http.php |
Introduces PHPStan parameter/return typing for URL parsing helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…content URL Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
The addition of PHPStan types here (cf. Core-64238) ensures that all of the scenarios are accounted for which could cause errors. |
See Trac comment for full description of issue: https://core.trac.wordpress.org/ticket/65015#comment:47
Trac ticket: https://core.trac.wordpress.org/ticket/65015
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 4.7
Used for: PHPStan type research
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.